adding dynamic swap fee to fx pools#1101
Conversation
🦋 Changeset detectedLatest commit: fbdf00b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
franzns
left a comment
There was a problem hiding this comment.
We should think about how a negative fee and APR affects our UI and if this is what we want to show
| // Replica of the subgraph logic: | ||
| // https://github.com/balancer/balancer-subgraph-v2/blob/60453224453bd07a0a3a22a8ad6cc26e65fd809f/src/mappings/vault.ts#L551-L564 | ||
| if (swap.poolId.poolType === 'FX') { | ||
| const USDC_ADDRESS = '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48'; |
There was a problem hiding this comment.
How can we hardcode this? Isnt this different for each chain?
There was a problem hiding this comment.
Do we only compare against USDC? No other FOREX such as EURC?
There was a problem hiding this comment.
It looks like all the markets are against USDC, but true that it needs to be done for other chains as well.
There was a problem hiding this comment.
Hi @franzns, @gmbronco, would it be possible to add a quoteToken property on the Pool model? This new prop will hold the quote token address, whether USDC, USDC.e, or any token.
This is how we assign the quoteToken value in our balancer-subgraph fork:
function handleNewFXPool(event: ethereum.Event, permissionless: boolean): void {
...
pool.quoteToken = getFXPoolQuoteToken(poolAddress);
...
}
function getFXPoolQuoteToken(poolAddress: Address): Bytes {
let poolContract = FXPool.bind(poolAddress);
// FXPool **always** holds the quote token in the second position
let call = poolContract.try_derivatives(BigInt.fromI32(1));
if (call.reverted) {
log.error('Failed to get quote token for FXPool: {}', [poolAddress.toHexString()]);
return stringToBytes('0x');
}
return call.value;
}
We then refer to quoteToken instead of hardcoded USDC addresses to determine base and quote:
let isTokenInBase = tokenOutAddress === pool.quoteToken;
There was a problem hiding this comment.
Hi @franzns, @gmbronco, would it be possible to add a
quoteTokenproperty on the Pool model? This new prop will hold the quote token address, whether USDC, USDC.e, or any token.This is how we assign the
quoteTokenvalue in our balancer-subgraph fork:function handleNewFXPool(event: ethereum.Event, permissionless: boolean): void { ... pool.quoteToken = getFXPoolQuoteToken(poolAddress); ... } function getFXPoolQuoteToken(poolAddress: Address): Bytes { let poolContract = FXPool.bind(poolAddress); // FXPool **always** holds the quote token in the second position let call = poolContract.try_derivatives(BigInt.fromI32(1)); if (call.reverted) { log.error('Failed to get quote token for FXPool: {}', [poolAddress.toHexString()]); return stringToBytes('0x'); } return call.value; }We then refer to
quoteTokeninstead of hardcoded USDC addresses to determine base and quote:let isTokenInBase = tokenOutAddress === pool.quoteToken;
Yes, we should definitely do that. We already have pool type specific data where we can add the quote token. Is the quote token immutable?
There was a problem hiding this comment.
Yes, the quote token is immutable for FX pools
There was a problem hiding this comment.
I added the quote token to our pool type specifc data set
| } | ||
| } | ||
|
|
||
| feeUSD = String(feeFloatUSD); |
There was a problem hiding this comment.
Think we talked about the option of a possible negative APR. Would that mean that his fee might be negative? What are the downstream effects if so?
There was a problem hiding this comment.
is there upside / downside? i assume it's just a property of FX pools. unless you mean if some other parts of the app will need to be updated. that i'm not sure. Is there anything that depends on the apr?
|
Hi @gmbronco, don't we also need to update how the The code assumes a static pool swap fee, but FX pools have a dynamic swap fee I'm thinking something like this might work: |
Good catch. We'll need to think about that as we'll change the snapshot generation not to rely on subgraph pricing anymore. We can calculate |
Great, that makes sense to me! |
08b5c00 to
dbfac77
Compare
| const update = async (data: { id: string; chain: Chain; typeData: any }[]) => { | ||
| // Update the pool type data | ||
| const updates = data.map(({ id, chain, typeData }) => | ||
| prisma.prismaPool.update({ | ||
| where: { id_chain: { id, chain } }, | ||
| data: { typeData }, | ||
| }), | ||
| ); | ||
|
|
||
| await prismaBulkExecuteOperations(updates, false); | ||
| }; |
There was a problem hiding this comment.
I dont think this works as a general approach. Some pool types will have mutable and immutable type data, meaning some will be synced at creation via subgraph and some will be synced via on-chain in regular intervals.
There was a problem hiding this comment.
how does it matter? do you mean to avoid unnecessary update calls?
There was a problem hiding this comment.
It actually doesnt really here as you handle FX pools outside of the "update type data flow".
| valueUSD: String((feeToken?.price || 0) * parseFloat(swap.payload.fee.amount)), | ||
| valueUSD: String(feeValueUSD > 0 ? feeValueUSD : swap.payload.fee.valueUSD), |
There was a problem hiding this comment.
I think you set it to swap.payload.fee.valueUSD because that is still 0 at this time? Think this is a but bogus, just set to 0?
There was a problem hiding this comment.
It's a passthrough for a default USD price coming from the subgraph, in case there is no pricing in the DB. It's set earlier and it important to the FX fee, because it's set based on the subgraph data of latestFx rates.
There was a problem hiding this comment.
Ah gotcha, this is v2. This is just used as fallback so its fine 👍
301f5cd to
fbdf00b
Compare
closes #802